Skip to content

fix(app): keep session composer clearance synced#375

Merged
Astro-Han merged 2 commits intodevfrom
pawwork/composer-dock-height-debug
May 2, 2026
Merged

fix(app): keep session composer clearance synced#375
Astro-Han merged 2 commits intodevfrom
pawwork/composer-dock-height-debug

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

@Astro-Han Astro-Han commented May 2, 2026

Summary

  • Keep session timeline clearance synchronized when the composer dock changes height after mount.
  • Increase the extra bottom clearance above the composer so the latest response sits higher.
  • Add regression coverage for dynamic prompt dock resizing and the composer clearance contract.

Why

The latest conversation turn could sit too close to the composer after #360. When image attachments or todo/follow-up docks increased composer height after initial mount, the timeline did not always update --composer-dock-height, so the conversation flow was not pushed upward correctly.

Related Issue

No linked issue. This follows up on the spacing regression noticed after #360.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

  • packages/app/src/pages/session/use-session-scroll-dock.ts: observer lifecycle for prompt/content refs and cleanup.
  • packages/app/src/pages/session/message-timeline.tsx: whether the extra 32px composer clearance feels right visually.
  • Regression test shape in use-session-scroll-dock.test.ts.

Risk Notes

Low UI layout risk. This changes session timeline spacing and ResizeObserver handling in the composer dock. No data, permissions, dependency, migration, packaging, signing, or updater changes.

How To Verify

Targeted tests: 11 passed, 0 failed (`bun test src/pages/session/use-session-scroll-dock.test.ts src/shell-frame-contract.test.ts`)
App typecheck: ok (`bun run typecheck`)
Manual Electron dev check: normal composer, image attachment composer, and todo dock spacing looked correct
Diff check: no unrelated generated snapshot changes included

Screenshots or Recordings

No screenshot attached. Manual Electron dev check was performed locally; screen capture was unavailable because macOS Screen Recording permission was not granted for the agent terminal.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, scope, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

  • Style
    • Adjusted padding in the session message timeline for improved visual spacing.

@Astro-Han Astro-Han added bug Something isn't working app Application behavior and product flows ui Design system and user interface labels May 2, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 608abe4f-3809-46a5-bd42-168972f0685e

📥 Commits

Reviewing files that changed from the base of the PR and between 07c4587 and aef0bf6.

📒 Files selected for processing (1)
  • packages/app/src/pages/session/use-session-scroll-dock.test.ts

📝 Walkthrough

Walkthrough

This PR switches scroll-dock resize handling to native ResizeObserver instances, updates the session message timeline bottom padding to account for the composer dock CSS variable, and adds test helpers and tests to validate that --composer-dock-height is set and updated on prompt dock resizes.

Changes

Session scroll-dock & layout

Layer / File(s) Summary
Core Implementation
packages/app/src/pages/session/use-session-scroll-dock.ts
Replaced the utility createResizeObserver usage with native ResizeObserver. Added contentObserver and promptDockObserver lifecycle management, observe/disconnect on ref changes, schedule scroll-state recalculation and call input.fill() on content resizes, and recompute dock height on prompt-dock resizes; disconnect observers in onCleanup.
Layout Wiring
packages/app/src/pages/session/message-timeline.tsx
Adjusted scroll content container padding-bottom from calc(var(--composer-dock-height, 0px) + 16px) to calc(var(--composer-dock-height, 0px) + 32px).
Test Infrastructure
packages/app/src/pages/session/use-session-scroll-dock.test.ts
Added makeMeasuredDiv(height) to simulate resizable elements and withResizeObserver(...) to mock globalThis.ResizeObserver for synchronous test-triggered resize events; import createRoot for mounting reactive scope.
Test Coverage / Assertions
packages/app/src/pages/session/use-session-scroll-dock.test.ts, packages/app/src/shell-frame-contract.test.ts
New test mounts createSessionScrollDock, sets prompt dock ref, asserts initial --composer-dock-height, triggers a synthesized resize, and asserts updated CSS variable. Contract test updated to assert message-timeline.tsx contains padding-bottom: calc(var(--composer-dock-height, 0px) + 32px) (plus minor selector formatting).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

🐰 I measured docks with gentle paws,
Observers listen without a pause,
Padding grows to make space right,
Scroll and composer sleep at night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(app): keep session composer clearance synced' accurately summarizes the main change: keeping the session composer clearance synchronized when the composer dock changes height.
Description check ✅ Passed The pull request description follows the template structure, including all major sections: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, Screenshots or Recordings, and a completed Checklist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pawwork/composer-dock-height-debug

Review rate limit: 7/10 reviews remaining, refill in 13 minutes and 5 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Astro-Han Astro-Han added the P2 Medium priority label May 2, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app/src/pages/session/use-session-scroll-dock.test.ts`:
- Around line 259-263: The cleanup restores the CSS var before calling
dispose(), but createSessionScrollDock's dispose() removes the variable and can
override restoration; change the order so you call dispose() first and then
restore or remove the "--composer-dock-height" using previousDockHeight and
document.documentElement.style.setProperty/removeProperty (i.e., call dispose()
before applying previousDockHeight or removing the property).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88d0d28e-e32d-4622-8452-67fafc13b264

📥 Commits

Reviewing files that changed from the base of the PR and between edff69e and 07c4587.

📒 Files selected for processing (4)
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/use-session-scroll-dock.test.ts
  • packages/app/src/pages/session/use-session-scroll-dock.ts
  • packages/app/src/shell-frame-contract.test.ts

Comment thread packages/app/src/pages/session/use-session-scroll-dock.test.ts
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the @solid-primitives/resize-observer dependency with manual ResizeObserver management in use-session-scroll-dock.ts to better handle element reference updates. It also increases the bottom padding of the message timeline to 32px and introduces new test utilities and a test case to verify dynamic height updates of the composer dock. I have no feedback to provide.

@Astro-Han Astro-Han merged commit 0d8cae9 into dev May 2, 2026
27 checks passed
@Astro-Han Astro-Han deleted the pawwork/composer-dock-height-debug branch May 2, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows bug Something isn't working P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant